Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove setpoints and wind condition specifics from calculate_XX_plane methods #868

Merged
merged 12 commits into from
Apr 8, 2024

Conversation

misi9170
Copy link
Collaborator

@misi9170 misi9170 commented Apr 5, 2024

Various methods used for visualization on FlorisModel (calculate_horizontal_plane(), calculate_cross_plane(), and calculate_y_plane()) still accepted yaw_angles as an input, but not other control setpoints.

If yaw_angles were passed, they were applied; and if yaw_angles were not passed, the stored yaw angles were used (which could lead to findex length clashes, if the fmodel had n_findex > 1).

Simply removing yaw_angles as an input and allowing those that had already been set to be used could create a findex clash. Instead, I decided to:

  • remove yaw_angles as an input
  • also remove wd, ws, and ti as inputs
  • give the option to the user of which findex they would like to visualize
  • Added some infrastructure to enable this: new method set_for_viz; deepcopies of fmodel to get around issue with setpoints not being stored on the dictionary (which has come up a couple of times now---we may need a better fix for this issue).

The result works, but may be somewhat slower than before. However, I'm not sure this is the path we'd like to go down, so I'd like some feedback here. If we are OK with this, I can also make some changes to simplify calculate_horizontal_plane_with_turbines() on flow_visualization.py. Once those changes are in, the check_wind_condition_for_viz() method on FlorisModel can likely be removed. For now, I've simply removed the erroneous type hints on check_wind_condition_for_viz().

I'll also add some tests for all of this if we do want to go with this approach.

@misi9170 misi9170 added enhancement An improvement of an existing feature v4 Focus of FLORIS v4 labels Apr 5, 2024
@misi9170 misi9170 marked this pull request as draft April 5, 2024 02:39
@rafmudaf
Copy link
Collaborator

rafmudaf commented Apr 5, 2024

may be somewhat slower

What's the cause of this being slower?

@misi9170
Copy link
Collaborator Author

misi9170 commented Apr 5, 2024

may be somewhat slower

What's the cause of this being slower?

@rafmudaf , well, I'm not sure that it's slower, but I changed from exporting the existing fmodel to a dictionary and then reinstantiating to simply making a deepcopy of the exisiting fmodel. I think that's more robust (I did it in order to save the control setpoints), but I think more memory is needed for the deep copy than a configuration dictionary.

@misi9170 misi9170 self-assigned this Apr 7, 2024
@misi9170 misi9170 marked this pull request as ready for review April 7, 2024 21:52
@misi9170
Copy link
Collaborator Author

misi9170 commented Apr 7, 2024

I've now gone ahead and made the commensurate changes to calculate_horizontal_plane_with_turbines() and removed FlorisModel.check_wind_condition_for_viz(), which was made redundant.

floris/floris_model.py Outdated Show resolved Hide resolved
@misi9170 misi9170 merged commit 0e41d9b into NREL:v4 Apr 8, 2024
8 checks passed
@misi9170 misi9170 deleted the bugfix/remove-control-setpoints-viz branch April 8, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature v4 Focus of FLORIS v4
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants